Feature/query store timeslicer v1.1#151
Conversation
erikdarlingdata
left a comment
There was a problem hiding this comment.
Hey Romain, nice work on this — the quick filters and custom popup are solid additions. A few things I noticed:
Issues
1. Missing 48h quick filter
The PR description mentions "3h, 24h, 48h, 7d, 30d" but the AXAML only has buttons for 3h, 24h, 7d, 30d — looks like 48h got lost along the way.
2. Highlight behavior on manual drag/wheel
After any pointer drag or wheel zoom, _activeFilterTag resets to "0" which highlights the "Custom" button — even though the user didn't use the custom popup. It might be cleaner to have a "no highlight" state for manual range adjustments, so the Custom button only lights up when someone actually uses the popup.
3. MinNormInterval reduced to 1 bucket
The old code enforced a 3-hour minimum selection. The new code allows a 1-hour (single bucket) minimum. Is that intentional? Users could end up with a very narrow window that returns almost no data.
4. _activeFilterTag = "24" default vs. short data ranges
If the data has less than 24 hours, the 24h button still gets highlighted even though the actual selected range is smaller. The range calculation handles it correctly, but the visual indicator could be misleading.
5. Expand/collapse toggle removed
The old toggle to collapse the slicer is gone entirely. Some users may want to hide it to reclaim vertical space. Was that a deliberate decision?
6. Custom popup silently adjusts invalid ranges
If start > end, it quietly sets end = start + 1h with no feedback. A small validation message would be friendlier.
Nits
- The quick filter button lines in the AXAML are ~300 characters each — breaking attributes across multiple lines would help readability.
"periode"→"period"in the commit message (minor typo).AutoSelectTopN = 1changes the default from all-selected to top-1-selected — just want to confirm that's the intended UX change for all users.
Overall this is a great addition. I'd want the 48h button added and the highlight-on-drag behavior cleaned up before merging. The rest is minor. 👍
Second i would say that i test the app with real users that i let manipulate it and observe how they use it, blockers, questions ...and learn this way how to improve the app. I saw that the app is usefull. It help to find queries with high impact on the database performance using the query store and dig to plan This PR and features have been build after one session with a client of mine. I will work on it today and come back quickly |
|
@rferraton if you just want to put the 48hr in and skip the rest, that's fine with me. The rest is not much to worry about. It sounds like you have some market research that I do not have, ha ha ha. |
|
@erikdarlingdata : 48h button is back. |
What does this PR do?
Which component(s) does this affect?
How was this tested?
Describe the testing you've done. Include:
Checklist
dotnet build -c Debug)dotnet test)